Conversation
|
fixes #305 |
| const appJSON = await client.versions.exportMethod(appId, versionId) | ||
| if (appJSON) { | ||
| const outputPath = out ? out : '' | ||
| await utils.writeOutput(outputPath, appJSON, force) |
There was a problem hiding this comment.
Although it is very clean and helps to reduce code duplication having this on the utils file, I think it is easier to understand the command flow by keeping the flags processing in the command. e.g
if (flags.out) {
writeToFile()
} else {
writeToConsole()
}
packages/luis/src/utils/index.ts
Outdated
|
|
||
| const writeToConsole = (outputContents: string) => { | ||
| const output = JSON.stringify(outputContents, null, 2) | ||
| process.stdout.write('App successfully exported\n') |
There was a problem hiding this comment.
Please keep command responsibilities at the command level. e.g
// Command class
writeToConsole()
this.log(App successfully exported)
packages/luis/src/utils/index.ts
Outdated
| const validatedPath = utils.validatePath(outputLocation, '', force) | ||
| await fs.ensureFile(outputLocation) | ||
| await fs.writeJson(validatedPath, content, {spaces: 2}) | ||
| process.stdout.write(`File successfully written: ${validatedPath}`) |
There was a problem hiding this comment.
Please keep command responsibilities(logging) at the command level. e.g
// Command class
const path = writeToFile()
this.log(`File successfully written: ${path}`)
There was a problem hiding this comment.
I actually wanted to do that too, but with this model, the tests have a hard time reading the message since 'ctx.stdout' typically only reads the first line. need to think about how to overcome that:
expect(ctx.stdout).to.contain('File successfully written')
There was a problem hiding this comment.
This.log uses process.stdout.write under the hood
| @@ -36,17 +36,27 @@ const getLUISClient = (subscriptionKey: string, endpoint: string) => { | |||
| return luisClient | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I added this util function in order to limit the allowed config vars to 'appId, region, subscriptionKey, versionId' per Eyal's specs
* Adding luis:version:export cmd * Remove dependency * Update example * Only store certain values in config, per specs * Refactor / keep logging and flag parsing in cmd only * Refactor write file error handling
|
Fixes #47 |
No description provided.